Skip to content

[Analytics Hub] Use device timezone and calendar for stats requests #8316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 6, 2022
Merged

[Analytics Hub] Use device timezone and calendar for stats requests #8316

merged 1 commit into from
Dec 6, 2022

Conversation

@ealeksandrov
Copy link
Contributor

@ealeksandrov ealeksandrov commented Dec 5, 2022

Closes: #8277

Description

This PR updates time range logic to use timezone and calendar set on the current device.

How

Initial idea was to use datetime preferences from remote store configuration. But that resulted in differences from dashboard and widgets setup:

woocommerce-ios/WooCommerce/Classes/ViewRelated/Dashboard/Stats v4/StoreStatsAndTopPerformersViewController.swift

Lines 216 to 220 in 0340eb2

// Since the stats charts are based on site time zone, we set the time zone for the stats UI to be the site time zone.
// On the other hand, when syncing the stats data with the API, we want to use the device time zone to find the time range since the API date parameters
// have no time zone information and are relative to the site time zone (e.g. 12:00am-11:59pm for "Today" tab).
let timezoneForStatsDates = TimeZone.siteTimezone
let timezoneForSync = TimeZone.current

So I've switched to already adopted .current timezone & calendar usage.
If we decide to stick with store configuration, we need to update it at least in all 3 places.

Testing instructions

  1. Make sure you have paid orders on Monday and Sunday on your store.
  2. Build and run the app in debug/alpha mode.
  3. Switch to UK locale (first day of a week = Monday).
  4. On the dashboard screen make a note of "total revenue" value tap the "See more" button.
  5. On the analytics hub screen confirm that "total revenue" matches earlier value from the dashboard.
  6. Compare data for day/week/month ranges between the dashboard and the analytics hub.
  7. Switch to US locale (first day of a week = Sunday).
  8. Compare data for day/week/month ranges between the dashboard and the analytics hub.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@ealeksandrov ealeksandrov added the feature: stats Related to stats, including Top Performers. label Dec 5, 2022
@ealeksandrov ealeksandrov added this to the 11.6 milestone Dec 5, 2022
@ealeksandrov ealeksandrov requested a review from ThomazFB December 5, 2022 19:09
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 5, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8316-816676b on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@ThomazFB ThomazFB assigned ThomazFB and unassigned ThomazFB Dec 5, 2022
ThomazFB
ThomazFB approved these changes Dec 6, 2022
View reviewed changes
Copy link
Contributor

@ThomazFB ThomazFB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this investigation and solution, @ealeksandrov! If the entire app stats uses those calendar/timezone configurations, then I agree we're good in using these too for the Hub! :shipit:

@ealeksandrov ealeksandrov merged commit e8b4feb into trunk Dec 6, 2022
@ealeksandrov ealeksandrov deleted the issue/8277-timezones-config branch December 6, 2022 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ThomazFB ThomazFB ThomazFB approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ThomazFB ThomazFB

Labels

feature: stats Related to stats, including Top Performers.

Projects

None yet

Milestone

11.6

Development

Successfully merging this pull request may close these issues.

[Analytics Hub] Use store time zone and week day start configuration

4 participants

@ealeksandrov @wpmobilebot @ThomazFB